-
Notifications
You must be signed in to change notification settings - Fork 26.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avoid many failing tests in doctesting #27262
Conversation
utils/not_doctested.txt
Outdated
docs/source/en/tasks/asr.md | ||
docs/source/en/tasks/audio_classification.md | ||
docs/source/en/tasks/document_question_answering.md | ||
docs/source/en/tasks/idefics.md # cause other test failing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
allow to add comment (despite not very pretty)
@@ -54,7 +54,7 @@ def clean_doctest_list(doctest_file: str, overwrite: bool = False): | |||
all_paths = [] | |||
with open(doctest_file, "r", encoding="utf-8") as f: | |||
for line in f: | |||
line = line.strip() | |||
line = line.strip().split(" ")[0] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
take into account the potential comments
in some lines of utils/not_doctested.txt
removed_content = set(old_content.split("\n")) - set(new_content.split("\n")) | ||
removed_content = {x.split(" ")[0] for x in old_content.split("\n")} - { | ||
x.split(" ")[0] for x in new_content.split("\n") | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same: deal with the potential comments
utils/not_doctested.txt
Outdated
src/transformers/models/blip/modeling_tf_blip_text.py | ||
src/transformers/models/blip_2/configuration_blip_2.py | ||
src/transformers/models/blip_2/convert_blip_2_original_to_pytorch.py | ||
src/transformers/models/blip_2/modeling_blip_2.py # cause other test failing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
currently, src/transformers/models/blip_2/modeling_blip_2.py
and docs/source/en/tasks/idefics.md
cause problem to other tests
The documentation is not available anymore as the PR was closed or merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing!
utils/not_doctested.txt
Outdated
docs/source/en/tasks/asr.md | ||
docs/source/en/tasks/audio_classification.md | ||
docs/source/en/tasks/document_question_answering.md | ||
docs/source/en/tasks/idefics.md # cause other test failing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not able to parse this comment is it saying:
- Because other tests are failing
or - Causes other tests to fail
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these 2 files, if run in doctesting, cause other tests to fail.
I will change it to causes other tests to fail
* fix * update * update * fix --------- Co-authored-by: ydshieh <[email protected]>
What does this PR do?
Avoid many failing tests in doctesting: currently, src/transformers/models/blip_2/modeling_blip_2.py and docs/source/en/tasks/idefics.md cause problem to other tests
With this PR: the failing list is much shorter
Currently failing tests without this PR